-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Schedule polling for new meetings #12
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It took quite a while to get used to this style of code.
It looks like the summaries are generated from the agenda PDF? I thought we should generate summaries from the transcripts instead? Or we could have an agenda summary AND a transcript summary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change (blocking): don't check the *.jsonl
file into the repo or we'll have to git commit them into the repo to update them. They should be put into the storage destination like S3 or something.
SUMMARY_FILE = Path("data/summaries.jsonl") | ||
|
||
|
||
async def load_meetings() -> List[Meeting]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question (blocking): why is this async?
) | ||
|
||
# Generate the AI summary | ||
ai_summary = await summarize_with_llama(summary) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (non-blocking): If we're using aiohttp
to do these concurrently, shouldn't we remove the await
from this call? So the process can do all the summaries
concurrently? Or am I forgetting how async http and concurrency works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, a gather() does make sense here.
clean up meeting data, add action to save meetings
Experimenting with github action to get new meetings and write summaries